CSHARP-5603: Add Big Endian support in BinaryVectorReader and BinaryVectorWriter #1682

medhatiwari · 2025-05-05T10:53:54Z

Description

This PR adds Big Endian support for System.Single (Float32) to the BinaryVectorWriter.WriteToBytes() method.

Background

While running the MongoDB.Bson.Tests test suite on a Big Endian (s390x) system, we encountered 34 consistent test failures within the BinaryVectorSerializerTests class.
Each failure was caused by a System.NotSupportedException indicating that binary vector data of float32 type is not yet supported on Big Endian architectures.

Exception Observed

System.NotSupportedException: Binary vector data is not supported on Big Endian architecture yet.

Sample Failing Tests

Some of the test cases that failed due to this limitation include:

BinaryVectorSerializerTests.BinaryVectorSerializer_should_deserialize_bson_vector<Float32>

BinaryVectorSerializerTests.BinaryVectorSerializer_should_serialize_bson_vector<Float32>

BinaryVectorSerializerTests.ArrayAsBinaryVectorSerializer_should_deserialize_bson_vector<Float32>

BinaryVectorSerializerTests.ArrayAsBinaryVectorSerializer_should_serialize_bson_vector<Float32>

BinaryVectorSerializerTests.MemoryAsBinaryVectorSerializer_should_serialize_bson_vector<Float32>

BinaryVectorSerializerTests.MemoryAsBinaryVectorSerializer_should_deserialize_bson_vector<Float32>

BinaryVectorSerializerTests.ReadOnlyMemoryAsBinaryVectorSerializer_should_serialize_bson_vector<Float32>

BinaryVectorSerializerTests.ReadOnlyMemoryAsBinaryVectorSerializer_should_deserialize_bson_vector<Float32>

Why This Fix Is Necessary

This limitation was blocking test pass status on Big Endian platforms such as s390x. Adding support for float32 serialization in Big Endian format:

Enables consistent behavior across architectures

Completes existing deserialization support added earlier in BinaryVectorReader.cs

Changes Introduced

Added Big Endian branch to BinaryVectorWriter.WriteToBytes() for T == float.

Used BinaryPrimitives.WriteSingleBigEndian() to write bytes in the correct order.

Left existing Little Endian logic untouched to preserve behavior.

cc: @giritrivedi

…<T>() Signed-off-by: Medha Tiwari <[email protected]>

Signed-off-by: Medha Tiwari <[email protected]>

medhatiwari · 2025-05-06T09:31:34Z

Hi @BorisDog, if everything if fine, can this be merged?

medhatiwari · 2025-05-19T04:42:04Z

Hi @BorisDog, just following up to check if there's any update on this PR. Please let me know if any further changes are needed.

src/MongoDB.Bson/Serialization/BinaryVectorReader.cs

src/MongoDB.Bson/Serialization/BinaryVectorWriter.cs

src/MongoDB.Bson/Serialization/BinaryVectorReader.cs

BorisDog

Review is pending on requested changes.

…or float32 on all platforms Signed-off-by: Medha Tiwari <[email protected]>

…ation Signed-off-by: Medha Tiwari <[email protected]>

…ndling Signed-off-by: Medha Tiwari <[email protected]>

BorisDog

The tests fail on net472.

src/MongoDB.Bson/IO/BinaryPrimitivesCompat.cs

src/MongoDB.Bson/Serialization/BinaryVectorReader.cs

src/MongoDB.Bson/IO/BinaryPrimitivesCompat.cs

src/MongoDB.Bson/Serialization/BinaryVectorWriter.cs

tests/MongoDB.Bson.Tests/Serialization/Serializers/BinaryVectorSerializerTests.cs

src/MongoDB.Bson/IO/BinaryPrimitivesCompat.cs

Signed-off-by: Medha Tiwari <[email protected]>

BorisDog

Looks good! Tests are passing as well.
Few styling comments + tests improvement.

BorisDog · 2025-05-29T17:39:32Z

tests/MongoDB.Bson.Tests/IO/BinaryPrimitivesCompatTests.cs

+        public void ReadSingleLittleEndian_should_throw_on_insufficient_length()
+        {
+            var shortBuffer = new byte[3];
+            Assert.Throws<ArgumentOutOfRangeException>(() =>


Please switch to Record.Exception (some examples in BinaryVectorSerializerTests.cs)

tests/MongoDB.Bson.Tests/IO/BinaryPrimitivesCompatTests.cs

tests/MongoDB.Bson.Tests/Serialization/Serializers/BinaryVectorSerializerTests.cs

BorisDog · 2025-05-29T19:54:36Z

tests/MongoDB.Bson.Tests/Serialization/Serializers/BinaryVectorSerializerTests.cs

+            {
+                return MemoryMarshal.Cast<T, byte>(span).ToArray();
+            }
+            int elementSize = Marshal.SizeOf<T>();


Please use var where possible.

BorisDog · 2025-05-29T22:27:12Z

src/MongoDB.Bson/Serialization/BinaryVectorWriter.cs

-                throw new NotSupportedException("Binary vector data is not supported on Big Endian architecture yet.");
-            }
+                case BinaryVectorDataType.Float32:
+                    var length = vectorData.Length * sizeof(float);


we can just have vectorData.Length * 4.
Float32 format is defined as 32 bits , in all other places 4 is hardcoded.

BorisDog · 2025-05-29T22:29:57Z

src/MongoDB.Bson/Serialization/BinaryVectorWriter.cs

+                    resultBytes[1] = padding;
+
+                    var floatSpan = MemoryMarshal.Cast<TItem, float>(vectorData);
+                    Span<byte> floatOutput = resultBytes.AsSpan(2);


BorisDog · 2025-05-29T22:31:31Z

src/MongoDB.Bson/Serialization/BinaryVectorWriter.cs

@@ -35,15 +36,41 @@ public static byte[] WriteToBytes<TItem>(BinaryVector<TItem> binaryVector)
        public static byte[] WriteToBytes<TItem>(ReadOnlySpan<TItem> vectorData, BinaryVectorDataType binaryVectorDataType, byte padding)
            where TItem : struct
        {
-            if (!BitConverter.IsLittleEndian)
+            byte[] resultBytes;


should be defined in Float32 case.
Also can be simplified to result.

BorisDog · 2025-05-29T23:10:45Z

src/MongoDB.Bson/IO/BinaryPrimitivesCompat.cs

+#endif
+        }
+
+        // This layout trick allows safely reinterpreting float as int and vice versa.


No need for the comment.

tests/MongoDB.Bson.Tests/IO/BinaryPrimitivesCompatTests.cs

BorisDog

Few more minor comments.

BorisDog · 2025-05-30T16:07:36Z

src/MongoDB.Bson/Serialization/BinaryVectorWriter.cs

+                case BinaryVectorDataType.Float32:
+                    byte[] result;
+                    var length = vectorData.Length * 4; 
+                    result = new byte[2 + length];


var result = new byte[2 + length]; is sufficient.

BorisDog · 2025-05-30T17:16:53Z

src/MongoDB.Bson/Serialization/BinaryVectorReader.cs

+		 result = new float[count];
+		 for (int i = 0; i < count; i++)


Please use 4 whitespaces instead of tab.

BorisDog · 2025-05-30T17:22:26Z

tests/MongoDB.Bson.Tests/IO/BinaryPrimitivesCompatTests.cs

+                BinaryPrimitivesCompat.ReadSingleLittleEndian(shortBuffer));
+
+            exception.Should().BeOfType<ArgumentOutOfRangeException>();
+            exception.Message.Should().Contain("length");


Please use the following pattern:

var e = exception.Should().BeOfType<ArgumentOutOfRangeException>().Subject; e.ParamName.Should().Be("length");

and in WriteSingleLittleEndian_should_throw_on_insufficient_length as well.

Also this seems to be the reason for ReadSingleLittleEndian_should_throw_on_insufficient_length and WriteSingleLittleEndian_should_throw_on_insufficient_length failers on net472.

BorisDog · 2025-05-30T17:24:42Z

src/MongoDB.Bson/IO/BinaryPrimitivesCompat.cs

+#else
+            if (source.Length < 4)
+            {
+                throw new ArgumentOutOfRangeException(nameof(source), "Source span is too small to contain a float.");


nameof(source.Length)?

Signed-off-by: Medha Tiwari <[email protected]>

Add Big Endian Support for Float32 in BinaryVectorWriter.WriteToBytes…

ff89368

…<T>() Signed-off-by: Medha Tiwari <[email protected]>

medhatiwari requested a review from a team as a code owner May 5, 2025 10:53

medhatiwari requested review from rstam and removed request for a team May 5, 2025 10:53

BorisDog requested review from BorisDog and removed request for rstam May 5, 2025 20:14

medhatiwari force-pushed the binaryvectorsupport branch from 5cd9ca1 to a4384e3 Compare May 6, 2025 09:00

Added comments for clarity

2c2cae1

Signed-off-by: Medha Tiwari <[email protected]>

medhatiwari force-pushed the binaryvectorsupport branch from a4384e3 to 2c2cae1 Compare May 6, 2025 09:01

BorisDog requested changes May 22, 2025

View reviewed changes

medhatiwari requested a review from BorisDog May 26, 2025 06:01

BorisDog requested changes May 27, 2025

View reviewed changes

medhatiwari added 3 commits May 28, 2025 14:42

Fix BinaryVectorSerializerTests to generate little-endian test data f…

2c4a16a

…or float32 on all platforms Signed-off-by: Medha Tiwari <[email protected]>

Add BinaryPrimitivesCompat methods for float32 little-endian serializ…

0ee1694

…ation Signed-off-by: Medha Tiwari <[email protected]>

Add float32 BinaryVector serialization/deserialization with endian ha…

f43d935

…ndling Signed-off-by: Medha Tiwari <[email protected]>

medhatiwari requested a review from BorisDog May 28, 2025 12:52

BorisDog requested changes May 28, 2025

View reviewed changes

medhatiwari added 2 commits May 29, 2025 15:10

added tests for new methods in BinaryPrimitivesCompat

92b7ed2

Signed-off-by: Medha Tiwari <[email protected]>

resolved all the review comments

530ecda

Signed-off-by: Medha Tiwari <[email protected]>

medhatiwari force-pushed the binaryvectorsupport branch from ee0aa0a to 530ecda Compare May 29, 2025 13:16

medhatiwari requested a review from BorisDog May 29, 2025 14:37

BorisDog requested changes May 29, 2025

View reviewed changes

resolved all the comments

02579c1

medhatiwari requested a review from BorisDog May 30, 2025 09:49

BorisDog requested changes May 30, 2025

View reviewed changes

BorisDog changed the title ~~Add Big Endian Support for Float32 in BinaryVectorWriter.WriteToBytes<T>()~~ CSHARP-5603: Add Big Endian support in BinaryVectorReader and BinaryVectorWriter May 30, 2025

BorisDog added the improvement label May 30, 2025

another set of changes to resolve minor issues

c547a15

Signed-off-by: Medha Tiwari <[email protected]>

medhatiwari force-pushed the binaryvectorsupport branch from 4078bfa to c547a15 Compare May 30, 2025 19:00

medhatiwari requested a review from BorisDog May 30, 2025 19:02

CSHARP-5603: Add Big Endian support in BinaryVectorReader and BinaryVectorWriter #1682

Are you sure you want to change the base?

CSHARP-5603: Add Big Endian support in BinaryVectorReader and BinaryVectorWriter #1682

Conversation

medhatiwari commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Background

Exception Observed

Sample Failing Tests

Why This Fix Is Necessary

Changes Introduced

Uh oh!

medhatiwari commented May 6, 2025

Uh oh!

medhatiwari commented May 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

medhatiwari commented May 5, 2025 •

edited

Loading

BorisDog May 30, 2025 •

edited

Loading